Repository navigation
Content Types: Fix MovePropertyType orphaning the property when moving it to no group (closes #23481) - #23493
Conversation
|
Claude finished @AndyButland's task in 4m 29s —— View job PR ReviewTarget: Fixes a data-loss bug in
Suggestions
ApprovedThe fix is targeted and correct, mirrors the existing |
MovePropertyType orphaning the property when moving it to no group (closes #23481)
Hoist the constant expected-alias arrays into static readonly fields (CA1861) and drop the redundant interface cast on the content type builder result (CA1859). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Express the un-grouped case as an else-if rather than a nested conditional, so the assignment reads as a single two-way choice. Behaviour is unchanged. Also adds unit tests for the previously uncovered paths - unknown property type, a move between two groups, and a no-op move of an already un-grouped property - so the restructure is covered before and after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…collection PropertyGroup.PropertyTypes is nullable with a public setter that accepts null, so the previous null-conditional Add could no-op while MovePropertyType still returned true - removing the property from its old home and adding it nowhere, which is the same orphaning that then deletes it (and its content values) on save. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|



Description
IContentTypeBase.MovePropertyType(propertyTypeAlias, propertyGroupAlias)documents that anullgroup alias "moves the property back to 'generic properties' ie does not have a tab anymore". It didn't do that — it lost the property entirely.Fixes #23481
The problem
ContentTypeBasekeeps ungrouped property types in its ownPropertyTypeCollection(exposed asNoGroupPropertyTypes), andPropertyTypesis the union of that collection and every group's properties.MovePropertyTypeonly ever touched group collections:So with a
nulltarget the property was removed from its group and added nowhere, disappearing from bothPropertyTypesandNoGroupPropertyTypes.The mirror direction was also wrong: moving a property from no group into a group never removed it from
PropertyTypeCollection(the old-group lookup only searches groups), leaving it in both collections until the content type was reloaded.The fix
MovePropertyTypenow removes the property from wherever it currently lives and adds it to wherever it's going, in both directions:This mirrors
RemovePropertyGroupin the same class, which already de-groups correctly (nullsPropertyGroupIdand adds toPropertyTypeCollection).The second parameter is now
string?on both the interface and the implementation, sincenullwas always the documented input. This is an annotation-only change and binary compatible;ContentTypeBaseis the only implementer in the CMS, and the solution builds with no new nullability warnings.An ungrouped property type is an already-supported, already-reachable state —
RemovePropertyGroupdeliberately re-homes properties there, and the Management API reaches it viaContainerKey = null(the codebase calls these "orphaned" properties, covered byCan_Make_Properties_OrphanedandCan_Remove_Properties_Without_Container). The backoffice renders such properties under the root container, labelled "Generic".MovePropertyType(alias, null)was the one route to that state that was broken.Testing
Automated
Three integration tests added in
ContentTypeServiceTestsand three unit tests inContentTypeTests:Every behavioural test was confirmed to fail before the fix by reverting the change in
ContentTypeBaseand re-running.Manual
Verified on the local dev site via a scratch controller (not committed, but code below) that loads a document type by alias, calls
MovePropertyType, saves viaIContentTypeService.UpdateAsync, and dumps the groups plusNoGroupPropertyTypesbefore the move, after the move in memory, and after save-and-reload.To reproduce manually without it:
title.title, save.contentType.MovePropertyType("title", null)and save the content type.titleis gone from the document type in the backoffice and its value is gone from the content item. After:titleappears under the "Generic" tab with the group empty, and the content item keeps its value.MovePropertyType("title", "content")returns it to the group and clears it from "Generic".